I wonder if there's any documentation that relies on checking the response from this endpoint in the logs to debug an NTP sync issue or something similar. I have a vague memory of something like that. If anyone remembers, that documentation should be updated. Perhaps an announcement on Matrix might be good as well? I've certainly used it before if an omicron deployment was taking too long to start.

I'm kinda tempted to suggest we do something here to confirm we have exactly one NTP zone per sled (probably via asserting, since we came up with the plan and it should always be correct?). Maybe that would be overkill, since 0 or more-than-one NTP zone on a sled is going to cause problems pretty quickly anyway? Something like (using exactly_one() from itertools):

let ntp_addresses: Vec<_> = service_plan
    .services
    .iter()
    .map(|(_, sled_config)| {
        sled_config.zones.iter().filter_map(|zone_config| {
            // ...extract ntp_admin_addr...
        })
        .exactly_one()
        .expect("plan should specify one NTP zone per sled")
    })
    .collect();

Since both of these arms bind a field named address with the same type, I think you can combine them?

                        BlueprintZoneType::BoundaryNtp(
                            blueprint_zone_type::BoundaryNtp {
                                address, ..
                            },
                        ) 
                        | BlueprintZoneType::InternalNtp(
                            blueprint_zone_type::InternalNtp { address },
                        ) => {
                            let mut ntp_admin_addr = *address;
                            ntp_admin_addr.set_port(NTP_ADMIN_PORT);
                            Some(ntp_admin_addr)
                        }

I realize this is consistent with what was already here, but: by accepting a list of socket addrs, we end up rebuilding an NtpAdminClient for every sled inside every attempt of the retry_notify loop. Maybe we should take a &[NtpAdminClient] instead of a list of socket addrs, so we only create the clients once? (I have a vague memory of reqwest clients being relatively expensive to construct, but maybe we solved that some other way?)

🎉 Love getting rid of this.